[DRAFT] fix: deduplicate x-goog-api-client headers#17616
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces changes to handle and merge duplicate x-goog-api-client metadata headers in google-api-core and allows suppressing these metrics headers in google-auth's AuthMetadataPlugin. Feedback on the changes includes a recommendation to copy self._arbitrary_metadata to prevent shared state mutation, and a suggestion to simplify the _extract_metrics_header function into a single-pass loop for better readability and compatibility with general iterables.
| if self._metrics_values: | ||
| kwargs["metadata"] = [(client_info.METRICS_METADATA_KEY, self._metrics_values)] + self._arbitrary_metadata | ||
| else: | ||
| kwargs["metadata"] = self._arbitrary_metadata |
There was a problem hiding this comment.
Assigning self._arbitrary_metadata directly to kwargs["metadata"] can lead to unexpected side effects. Since _GapicCallable is a reusable wrapper called multiple times, any downstream code or transport layer that modifies kwargs["metadata"] in-place will mutate self._arbitrary_metadata for all subsequent calls.
To prevent shared state mutation, please copy the list using list(self._arbitrary_metadata).
| kwargs["metadata"] = self._arbitrary_metadata | |
| kwargs["metadata"] = list(self._arbitrary_metadata) |
| if not metadata: | ||
| return [], [] | ||
|
|
||
| for i, (key, val) in enumerate(metadata): | ||
| if key == client_info.METRICS_METADATA_KEY: | ||
| # Key located. Check the rest of the list for duplicate entries | ||
| arbitrary_metadata = list(metadata[:i]) | ||
| metric_values = [val] | ||
| for k, v in metadata[i+1:]: | ||
| if k == client_info.METRICS_METADATA_KEY: | ||
| metric_values.append(v) | ||
| else: | ||
| arbitrary_metadata.append((k, v)) | ||
| return arbitrary_metadata, metric_values | ||
| # No key found | ||
| return list(metadata), [] |
There was a problem hiding this comment.
The current implementation of _extract_metrics_header uses index slicing (metadata[:i] and metadata[i+1:]) and multiple loops. This can be simplified into a single-pass loop that is more readable, performs better, and is more robust because it works on any iterable (including generators/iterators) without raising a TypeError on slicing.
if not metadata:
return [], []
arbitrary_metadata = []
metric_values = []
for key, val in metadata:
if key == client_info.METRICS_METADATA_KEY:
metric_values.append(val)
else:
arbitrary_metadata.append((key, val))
return arbitrary_metadata, metric_values
1 participant
We currently populate the x-goog-api-client header multiple times, in different places in the stack (api-core, google-aith, user-provided, etc). While some backend systems are able to handle the duplicate headers, it can cause issues for others
This PR explores a potential fix:
b/477429588